Login: Several small UX fixes for Google login - #8022
Conversation
| let errorDescription: String | ||
| if (error as! NSError).code == WordPressComOAuthError.unknownUser.rawValue { | ||
| errorTitle = NSLocalizedString("Connected But…", comment:"Title shown when a user logs in with Google but no matching WordPress.com account is found") | ||
| errorDescription = NSLocalizedString("The Google account \"\(loginFields.username)\" doesn't match any account on WordPress.com", comment:"D shown when a user logs in with Google but no matching WordPress.com account is found") |
There was a problem hiding this comment.
Colon Violation: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)
| let errorTitle: String | ||
| let errorDescription: String | ||
| if (error as! NSError).code == WordPressComOAuthError.unknownUser.rawValue { | ||
| errorTitle = NSLocalizedString("Connected But…", comment:"Title shown when a user logs in with Google but no matching WordPress.com account is found") |
There was a problem hiding this comment.
Colon Violation: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)
aerych
left a comment
aerych
left a comment
There was a problem hiding this comment.
Hiya @nheagy !
Just had a nitpick or two.
The only thing that would be a blocker is the comment about the responseObject vs responseDict. There's no compile error with this change because responseObject is an instance method... which means a function rather than a dictionary is being passed to errorFor(errorCode:errorDescription:responseObject:newNonce:)
|
|
||
| error?.pointee = errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject, newNonce: newNonce) | ||
| } | ||
| if [400, 409, 403].contains(httpResponse.statusCode), |
| var newNonce: String? = nil | ||
|
|
||
| // there's either a data object, or an error. | ||
| if let errorStr = responseDict["error"] as? String { |
| private func parseError(from responseDict: [String: AnyObject]) -> NSError { | ||
| var errorCode: String = "" | ||
| var errorDescription: String = "" | ||
| var newNonce: String? = nil |
There was a problem hiding this comment.
Nitpicky, but we could drop the type declarations for string :String since the assignment will take care of that via inference.
In the case of the optional, it would be assigned nil by default.
| let errorDescription: String | ||
| if (error as NSError).code == WordPressComOAuthError.unknownUser.rawValue { | ||
| errorTitle = NSLocalizedString("Connected But…", comment: "Title shown when a user logs in with Google but no matching WordPress.com account is found") | ||
| errorDescription = NSLocalizedString("The Google account \"\(loginFields.username)\" doesn't match any account on WordPress.com", comment: "D shown when a user logs in with Google but no matching WordPress.com account is found") |
There was a problem hiding this comment.
comment: "D shown ...
What's the D for?
There was a problem hiding this comment.
lol, I don't know why I didn't type escription after that 😂
| newNonce = data["two_step_nonce"] as? String | ||
| } | ||
|
|
||
| return errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject, newNonce: newNonce) |
There was a problem hiding this comment.
With the changes here I think we need to pass responseDict instead of responseObject.
|
Thanks for finding those, @aerych. Should be ready for another look 😀 |
|
Looks good :) Buddybuild failed due to a compile error with something that looked Aztec related. I restarted. Assuming all checks are green we're good to |
:)
|
Something seems off with buddybuild. All tests pass locally when I try. I think we're fine to just merge. |
|
Thanks @aerych 👍 |
3 participants
Refs #7675
To test:
Needs review: @aerych